Replace `COA.call` with `COA.callWithSigAndArgs` for reduced computation cost by m-Peter · Pull Request #77 · onflow/FlowYieldVaultsEVM

m-Peter · 2026-03-20T11:00:15Z

Related: onflow/flow-go#8401
Closes: #76

Description

Previously, to perform contract calls using a COA, it was necessary to produce the callData on Cadence side:

let calldata = EVM.encodeABIWithSignature(signature, args)

To be able to ABI-decode the returned data from the COA call, another step was needed:

let decoded = EVM.decodeABI(types: [Type<[UInt256]>()], data: res.data)
let amountsOut = decoded[0] as! [UInt256]

After doing some profiling on Cadence transactions, the computation cost of ABI encoding/decoding, amounted to about 15%-20% of the total computation, which is not trivial.

To optimize this case, we have a new wrapper function, callWithSigAndArgs, which does all this more efficiently:

coa.callWithSigAndArgs(
    to: to,
    signature: "approve(address,uint256)",
    args: [self.routerAddress, evmAmountIn],
    gasLimit: gasLimit,
    value: valueBalance.attoflow,
    resultTypes: [Type<[UInt256]>()]
)

We do the same for dryCall, by replacing it with dryCallWithSigAndArgs .

NOTE: CI is failing because we still need to deploy the EVM system contract on testnet/mainnet, for these new functions to be available.

…cost

claude · 2026-03-23T10:54:09Z

cadence/contracts/FlowYieldVaultsEVM.cdc


            if result.status != EVM.Status.successful {
-                let errorMsg = FlowYieldVaultsEVM.decodeEVMError(result.data)
+                let errorMsg = FlowYieldVaultsEVM.decodeEVMError(result.results as! [UInt8])


Unverified type assumption in all 13 error paths

Every failure branch in this PR casts result.results as! [UInt8] (13 sites across the contract). The old code used result.data, which was a dedicated [UInt8] field guaranteed to hold raw revert bytes. The new callWithSigAndArgs/dryCallWithSigAndArgs API stores results in [AnyStruct] — a field shared by both success (decoded Cadence values) and failure (raw revert bytes).

The downcast result.results as! [UInt8] succeeds only if the dynamic type of results on failure is [UInt8] at the array level. If the implementation wraps the bytes as a single element (e.g. [[UInt8]]) or populates results element-by-element from [AnyStruct], the downcast panics at runtime with a Cadence type error instead of decoding and propagating the actual EVM error.

CI is explicitly broken because callWithSigAndArgs isn't deployed yet, so this assumption is currently untested. Before merging, please add at least one Cadence test that exercises a failing EVM call via the new API and confirms that result.results as! [UInt8] correctly contains the revert data.

claude · 2026-03-23T10:54:17Z

cadence/contracts/FlowYieldVaultsEVM.cdc

-            let vaultIdentifiers = decoded[9] as! [String]
-            let strategyIdentifiers = decoded[10] as! [String]
+            if callResult.status != EVM.Status.successful {
+                let errorMsg = FlowYieldVaultsEVM.decodeEVMError(callResult.results as! [UInt8])


Graceful nil return replaced with potential panic in critical scheduler path

In the old code decodeEVMError(callResult.data) was safe — data was always [UInt8]. Here callResult.results as! [UInt8] may panic if the API's failure representation isn't a bare [UInt8] (see discussion on line 994).

The consequence here is worse than in state-mutating paths: getPendingRequestsFromEVM is called by the SchedulerHandler to decide which requests to process. Old behaviour on EVM call failure: emit ErrorEncountered, return nil, SchedulerHandler skips gracefully. New behaviour if the cast panics: the entire SchedulerHandler transaction reverts, crash recovery must handle it, and the root cause (EVM call failure + reason) is lost.

Additionally, resultTypes here includes Type<[UInt8]>() (for requestTypes and statuses). If the EVM runtime tries to decode even on failure, results could be partially or incorrectly populated — making the as! [UInt8] cast semantically ambiguous on the failure path. Consider using a dedicated helper that checks callResult.status first and accesses raw error bytes via a well-typed accessor rather than a force-cast on the shared results field.

claude · 2026-03-23T10:56:38Z

Review: Replace `COA.call` with `COA.callWithSigAndArgs`

The optimization goal is sound. Eliminating the separate encodeABIWithSignature and decodeABI steps for a 15-20% computation saving is worthwhile. The refactor is mechanically consistent across all 11 files. Two concrete issues need resolution before merge.

Issue 1: Untested type assumption in every failure path (13 sites) -- High

Every if result.status != EVM.Status.successful block now does:

let errorMsg = FlowYieldVaultsEVM.decodeEVMError(result.results as! [UInt8])

The old API had a dedicated result.data: [UInt8] field -- always a flat byte array on success or failure. The new API stores everything in result.results: [AnyStruct] -- decoded Cadence values on success and (presumably) raw revert bytes on failure. The downcast result.results as! [UInt8] succeeds only if the dynamic type of results on failure is [UInt8] at the array level. If the Flow EVM implementation wraps the bytes differently (e.g. appends them element-by-element into [AnyStruct], or nests them as [[UInt8]]), every single error path panics with a Cadence runtime type error instead of decoding and reporting the actual EVM failure reason.

CI is explicitly broken because callWithSigAndArgs is not yet deployed, so this assumption is completely untested. The behavioural contract of the new API on failure must be confirmed -- ideally via a Cadence test that exercises a reverting EVM call and asserts that result.results as! [UInt8] yields the expected revert bytes -- before this lands.

Inline comment: #77 (comment)

Issue 2: `getPendingRequestsFromEVM` -- graceful nil replaced by potential panic in scheduler critical path -- Medium

In the old code, decodeEVMError(callResult.data) was unconditionally safe -- data was always [UInt8]. In the new code, if the results as! [UInt8] cast fails on an EVM call failure, the function panics instead of emitting ErrorEncountered and returning nil.

The consequence matters because this function drives SchedulerHandler batch selection. The old degraded behaviour (return nil, skip scheduling) is replaced by a transaction revert requiring crash recovery. The root cause (EVM call failure and reason) is also lost.

Additionally, the resultTypes list for this call includes Type<[UInt8]>() at indices 2 and 3 (for requestTypes/statuses). On failure, the entire results is cast to [UInt8]. These are semantically different uses of the same field -- correctness relies on the runtime choosing a completely different dynamic type for results in the failure case vs. the success case.

Inline comment: #77 (comment)

Other observations (no action needed)

value: refundValue.attoflow in completeProcessing: Correct -- .attoflow extracts UInt from EVM.Balance, matching the new API parameter type.
assert(callResult.results.length == N, ...): Slightly stricter than the old silent decode-and-crash; equivalent in practice given transaction atomicity.
Scripts (get_allowlist_status, get_evm_contract_config, etc.): Mechanically correct; failure paths in scripts do not call decodeEVMError, so the cast risk is isolated to the contract.
resultTypes: nil for void calls: Appropriate for state-mutating calls with no meaningful return value.

m-Peter self-assigned this Mar 20, 2026

m-Peter force-pushed the mpeter/optimize-evm-call-usage branch from ae6fa70 to 12290b2 Compare March 20, 2026 11:54

Replace COA.call with COA.callWithSigAndArgs for reduced computation …

85dc5f3

…cost

m-Peter force-pushed the mpeter/optimize-evm-call-usage branch from 12290b2 to 85dc5f3 Compare March 20, 2026 12:09

m-Peter marked this pull request as ready for review March 23, 2026 10:48

m-Peter requested review from liobrasil and nvdtf March 23, 2026 10:48

claude bot reviewed Mar 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `COA.call` with `COA.callWithSigAndArgs` for reduced computation cost#77

Replace `COA.call` with `COA.callWithSigAndArgs` for reduced computation cost#77
m-Peter wants to merge 1 commit intomainfrom
mpeter/optimize-evm-call-usage

m-Peter commented Mar 20, 2026

Uh oh!

claude bot Mar 23, 2026

Uh oh!

claude bot Mar 23, 2026

Uh oh!

claude bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

m-Peter commented Mar 20, 2026

Description

Uh oh!

claude bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review: Replace COA.call with COA.callWithSigAndArgs

Issue 1: Untested type assumption in every failure path (13 sites) -- High

Issue 2: getPendingRequestsFromEVM -- graceful nil replaced by potential panic in scheduler critical path -- Medium

Other observations (no action needed)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude bot commented Mar 23, 2026 •

edited

Loading

Review: Replace `COA.call` with `COA.callWithSigAndArgs`

Issue 2: `getPendingRequestsFromEVM` -- graceful nil replaced by potential panic in scheduler critical path -- Medium